Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for new record types (cont.) #2118

Merged
merged 8 commits into from
Nov 28, 2022

Conversation

antoniusanggito
Copy link
Contributor

@antoniusanggito antoniusanggito commented Nov 18, 2022

Description

  • Source branch in your fork has meaningful name (not main)

Fixes #2062
Continuing from pull request #2099

Before Merge Checklist

These items can be completed after PR is created.

(Check any items that are not applicable (NA) for this PR)

  • JavaScript implementation
  • Python implementation (NA if HTML beautifier)
  • Added Tests to data file(s)
  • [NA] Added command-line option(s) (NA if
  • [NA] README.md documents new feature/option(s)

@bitwiseman
Copy link
Member

bitwiseman commented Nov 20, 2022

You might do better If you changed the tokenizer to recognize #{ as a START_BLOCK token the same as {. I don't know of any case where we would want to separate #{ wheb the user enters them together.

If #{ occurs at a point where the record syntax would not be valid, I think we still want to treat it as though it were valid to avoid bad behavior. We already do something like this for object syntax - If we see { with an identifier followed by : we treat it as an object even if that isn't valid syntax at that point in the code.

@antoniusanggito
Copy link
Contributor Author

antoniusanggito commented Nov 24, 2022

The checklists are done now. Let me know for any updates from your review @bitwiseman.

Sorry for a bit messy history while I was trying to rename the commit message.

@antoniusanggito
Copy link
Contributor Author

The make commands are passed on my end now

@bitwiseman
Copy link
Member

Delete the .DS_store file, please.

@antoniusanggito
Copy link
Contributor Author

Done, brought it back.. it was deleted accidentally

@bitwiseman bitwiseman merged commit 12bc378 into beautifier:main Nov 28, 2022
@bitwiseman
Copy link
Member

Well done! Thanks!

@bitwiseman bitwiseman added this to the v1.14.8 (next release) milestone Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Erroneous output with upcoming Records syntax
3 participants